Skip to content

tests/ui: A New Order [14/N] #142440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

tests/ui: A New Order [14/N] #142440

wants to merge 10 commits into from
+209 −194

Conversation

Kivooeo
Copy link
Contributor

@Kivooeo Kivooeo commented Jun 12, 2025 •
edited
Loading

Note

Intermediate commits are intended to help review, but will be squashed prior to merge.

Some tests/ui/ housekeeping, to trim down number of tests directly under tests/ui/. Part of #133895.

r? @jieyouxu

@rustbot
Copy link
Collaborator

rustbot commented Jun 12, 2025

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-tidy Area: The tidy tool S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 12, 2025
@rustbot rustbot assigned jieyouxu and unassigned lcnr Jun 12, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
fmt check
fmt: checked 6059 files
tidy check

thread 'deps (.)' panicked at src/tools/tidy/src/deps.rs:594:24:
cmd.exec() failed with `cargo metadata` exited with an error:     Updating crates.io index
    Updating git repository `https://github.com/rust-lang/team`
error: failed to get `rust_team_data` as a dependency of package `site v0.1.0 (/checkout/src/tools/rustc-perf/site)`

Caused by:
  failed to load source for dependency `rust_team_data`

Caused by:
  Unable to update https://github.com/rust-lang/team#a5260e76

Caused by:
  failed to clone into: /cargo/git/db/team-679b79b2e58466cd

Caused by:
  revision a5260e76d3aa894c64c56e6ddc8545b9a98043ec not found

Caused by:
---
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

thread 'main' panicked at src/tools/tidy/src/main.rs:62:49:
called `Result::unwrap()` on an `Err` value: Any { .. }
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:01:52
  local time: Thu Jun 12 23:48:59 UTC 2025
  network time: Thu, 12 Jun 2025 23:48:59 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@speedy-lex
Copy link
Contributor

@Kivooeo, Why is this refactor split up into 14 pr’s? Wouldn’t it be better for it to be in 1 pr?

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jun 13, 2025 •
edited
Loading

@speedy-lex

This is part of a long-term cleanup initiative tracked in #133895

The PRs are intentionally small — around 5–7 tests each — to make them easier to review and avoid overwhelming maintainers

Kivooeo
Kivooeo commented Jun 13, 2025
View reviewed changes
tests/ui/issue-15924.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: i mistakenly deleted issue-15924.rs (#15924) thinking it didn't reproduce the original problem. Should I restore it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only reason i'm asking is because of this comment #15924 (comment), seems like there is no way to reproduce this anymore because of "new interface scheme", making the regression test potentially obsolete

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, regression tests should err on the side of sticking around. Not being able to reproduce is a good thing - but just because the code is refactored doesn't mean a new design can't hit the same problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so preferably to keep this test?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO a solid choice when in doubt :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure! thanks for the answer ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgross35 tgross35 tgross35 left review comments

Assignees

@jieyouxu jieyouxu

Labels
A-tidy Area: The tidy tool S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Kivooeo @rustbot @rust-log-analyzer @speedy-lex @tgross35 @lcnr @jieyouxu